Skip to content

fix(#1220): add fix role to PerRepoDefaultRoles for dispatch loop#1225

Merged
rh-hemartin merged 2 commits into
mainfrom
fullsend/code/issue-1220
May 28, 2026
Merged

fix(#1220): add fix role to PerRepoDefaultRoles for dispatch loop#1225
rh-hemartin merged 2 commits into
mainfrom
fullsend/code/issue-1220

Conversation

@fullsend-ai-coder
Copy link
Copy Markdown

The fix role was missing from PerRepoDefaultRoles(), causing the dispatch role-check to block fix agent runs after review requested changes. All other agent roles were already included. The fix role has full agent scaffolding and is ready for production use.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com


Closes #1220

Post-script verification

  • Branch is not main/master (fullsend/code/issue-1220)
  • Secret scan passed (gitleaks — ece4b4b71a4d2eb5563ac365f04a333b95722288..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 19, 2026

Site preview

Preview: https://1d0d0082-site.fullsend-ai.workers.dev

Commit: abf9ad1718cec8ff1bba4c9290b0f1eceaf40b42

@fullsend-ai-review
Copy link
Copy Markdown

fullsend-ai-review Bot commented May 19, 2026

Review

Findings

Low / Info

  • [correctness] internal/config/config.go:70DefaultAgentRoles() also excludes fix, meaning org-mode installations via fullsend admin install default to a defaults.roles list without fix. The same dispatch-blocking issue from Add fix role to PerRepoDefaultRoles so review→fix loop works by default #1220 may occur in org mode. This is out of scope for this per-repo-targeted fix but should be tracked as follow-up. Note: DefaultAgentRoles() is overloaded for both app creation (where excluding fix is correct — it reuses the coder app) and config role population (where fix should be present). A separate function or explicit config-role augmentation may be needed.
    Remediation: File a follow-up issue to audit DefaultAgentRoles() usage paths and ensure org-mode configs include fix in defaults.roles without creating a spurious fix GitHub App.

  • [documentation-currency] docs/guides/admin/github-setup.md:114 — The --agents flag documentation describes the per-repo default as "(per-repo omits fullsend)" but after this PR the per-repo default also adds fix compared to org-mode. The parenthetical is now incomplete.
    Remediation: Update the description to note the per-repo default is triage,coder,review,fix,retro,prioritize.

Previous run

Review

Findings

No findings.

Previous run (2)

Review

Findings

Low

  • [test-coverage] internal/config/config_test.go — No dedicated test for PerRepoDefaultRoles(). Other role functions (ValidRoles, DefaultAgentRoles) have explicit tests, but PerRepoDefaultRoles() lacks one. The linked issue's triage comment proposed a test; adding one would prevent future regressions where a role is accidentally omitted again.
    Remediation: Add a TestPerRepoDefaultRolesIncludesFix test (or a broader test asserting all expected roles are present), similar to the pattern in TestValidRoles.

@fullsend-ai-review
Copy link
Copy Markdown

Review follow-ups

Created follow-up issues for actionable non-blocking review findings:

  • #1226 — No dedicated test for PerRepoDefaultRoles(). Other role functions (ValidRoles, DefaultAgentRoles) have explicit tests, but PerRepoDefaultRoles() lacks one. Adding a test would prevent future regressions where a role is accidentally omitted.

@rh-hemartin
Copy link
Copy Markdown
Contributor

/fs-fix

@rh-hemartin rh-hemartin self-assigned this May 20, 2026
@rh-hemartin rh-hemartin changed the title fix(#1220): add fix role to PerRepoDefaultRoles for review→fix dispatch loop fix(#1220): add fix role to PerRepoDefaultRoles for dispatch loop May 20, 2026
@rh-hemartin
Copy link
Copy Markdown
Contributor

/fs-fix

fullsend-ai-coder Bot pushed a commit that referenced this pull request May 21, 2026
Adds a dedicated test for PerRepoDefaultRoles() to verify all expected
roles are present and that the "fullsend" dispatch role is excluded in
per-repo mode. Prevents future regressions where a role is accidentally
omitted.

Addresses review feedback on #1225

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
@fullsend-ai-coder
Copy link
Copy Markdown
Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 1 review finding: added TestPerRepoDefaultRoles test for regression coverage of the PerRepoDefaultRoles() function.

Fixed (1):

  1. No dedicated test for PerRepoDefaultRoles() (internal/config/config_test.go): Added TestPerRepoDefaultRoles test that verifies all 6 expected roles are present (triage, coder, review, fix, retro, prioritize) and that the fullsend dispatch role is excluded in per-repo mode. Follows the same pattern as TestValidRoles.

Tests: passed

Updated by fullsend fix agent

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 21, 2026
@ralphbean ralphbean requested a review from ben-alkov May 26, 2026 18:44
Copy link
Copy Markdown
Contributor

@ben-alkov ben-alkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

fullsend-code and others added 2 commits May 28, 2026 07:51
The fix role was missing from PerRepoDefaultRoles(), causing the
dispatch role-check to block fix agent runs after review requested
changes. All other agent roles were already included. The fix role
has full agent scaffolding and is ready for production use.

Closes #1220

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a dedicated test for PerRepoDefaultRoles() to verify all expected
roles are present and that the "fullsend" dispatch role is excluded in
per-repo mode. Prevents future regressions where a role is accidentally
omitted.

Addresses review feedback on #1225

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 28, 2026
Merged via the queue into main with commit 8480e16 May 28, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add fix role to PerRepoDefaultRoles so review→fix loop works by default

2 participants